refactor: improve transcription UX and DX#1685
refactor: improve transcription UX and DX#1685drHuangMHT wants to merge 14 commits intomoeru-ai:mainfrom
Conversation
⏳ Approval required for deploying to Cloudflare Workers (Preview) for stage-web.
Hey, maintainers, kindly take some time to review and approve this deployment when you are available. Thank you! 🙏 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors transcription and auto-send logic by extracting it from ChatArea.vue into a new reusable composable, useTranscriptions.ts. This composable is then integrated into both ChatArea.vue and MobileInteractiveArea.vue to provide consistent transcription behavior across layouts. Review feedback suggests removing a redundant try-catch block in the new composable and transitioning from string-prefixed log messages to programmatic identifiers in metadata for better extensibility.
|
blocked by #1569 |
lietblue
added
feature
drHuangMHT
force-pushed
the
use-transcriptions
branch
from
647138c to
ffa9441
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f09a8e2ea1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| clearPendingAutoSend() | ||
| await stopStreamingTranscription(true) |
There was a problem hiding this comment.
Stopping transcription now cancels the debounce timer unconditionally and never flushes the last pending auto-send, so if a user turns the microphone off during the debounce window their final dictated sentence is left unsent. The previous logic explicitly sent pending text on stop; this refactor drops that behavior and can silently lose expected auto-send delivery in normal use.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
intended behavior
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cc7a1f169
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| watch([enabled], () => { | ||
| setupAnalyzer() | ||
| }, { immediate: true }) |
There was a problem hiding this comment.
setupAnalyzer() now depends on hearingPopoverOpen and stream, but the watcher only observes enabled. If the mic is already enabled, opening the hearing popover (or receiving the stream slightly later) no longer re-initializes the analyzer, so the volume visualization can stay inactive until enabled is toggled again.
Useful? React with 👍 / 👎.
drHuangMHT
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9915f883a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| watch([hearingDialogOpen, enabled, stream], () => { | ||
| watch([enabled], () => { |
There was a problem hiding this comment.
setupAnalyzer() still gates on hearingDialogOpen and stream, but the watcher now only tracks enabled. If the mic is already enabled, opening the hearing dialog or receiving the stream later will no longer re-run setup, so the volume meter can remain inactive until enabled is toggled again. This is a regression from the previous dependency set and affects normal UI interaction on mobile.
Useful? React with 👍 / 👎.
| }) | ||
|
|
||
| it('should expose startListening and stopListening', () => { | ||
| const { startListening, stopListening } = useTranscriptions(createOptions()) |
There was a problem hiding this comment.
The tests destructure startListening/stopListening, but useTranscriptions() now returns startStreamingTranscription/stopStreamingTranscription. As written, these variables are undefined, so calls like await startListening() will throw and make the new test suite fail instead of validating behavior.
Useful? React with 👍 / 👎.
2 participants
Description
HearingConfigis reused between popover and drawer interfaces.HearingConfignow supports toggling auto-send and transcription on and off.Linked Issues
Additional Context